Skip to content

Handle a mergedBy NaN value#165

Merged
nabobalis merged 1 commit intoexecutablebooks:mainfrom
jasongrout:typeerror
Jan 21, 2026
Merged

Handle a mergedBy NaN value#165
nabobalis merged 1 commit intoexecutablebooks:mainfrom
jasongrout:typeerror

Conversation

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jan 21, 2026 •
edited
Loading

This avoids a TypeError when retrieving PRs that have not been merged, which was causing a critical error. From where mergedBy is set, it can be a pandas na or it can be a username.

Fixes #164

To reproduce:

github-activity jupyterlab/jupyterlab --since v4.6.0a1 --kind pr

Error:

Traceback (most recent call last):
  File "/[snip]/github-activity", line 7, in <module>
    sys.exit(main())
             ~~~~^^
  File "/[snip]/github-activity/github_activity/cli.py", line 234, in main
    md = generate_activity_md(
        args.target,
    ...<3 lines>...
        **common_kwargs,
    )
  File "/[snip]/github-activity/github_activity/github_activity.py", line 594, in generate_activity_md
    data.at[ix, "contributors"] = list(item_contributors)
                                  ~~~~^^^^^^^^^^^^^^^^^^^
  File "/[snip]/github-activity/github_activity/github_activity.py", line 386, in __iter__
    for item in sorted(self.other - {self.author}):
                ~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '<' not supported between instances of 'float' and 'str'

@jasongrout
Handle a mergedBy NaN value
823bf88 
This avoids a TypeError when retrieving PRs that have not been merged, which was causing a critical error. From where mergedBy is set, it can be a pandas na or it can be a username.

Fixes executablebooks#164
@jasongrout
Copy link
Contributor Author

jasongrout commented Jan 21, 2026

What is weird to me is that this seems to only have recently started failing. Why did things pass before?

@bsipocz
Copy link
Member

bsipocz commented Jan 21, 2026

What is weird to me is that this seems to only have recently started failing. Why did things pass before?

Some upstream version differences?

@nabobalis nabobalis merged commit 1c406e7 into executablebooks:main Jan 21, 2026
5 checks passed
@krassowski
Copy link

krassowski commented Jan 21, 2026

Some upstream version differences?

It would be pandas v3.0.0 I bet. The timing agrees with when it started failing, and the changeset here looks relevant.

@choldgraf
Copy link
Member

choldgraf commented Jan 21, 2026

hah I just ran into this in myst-theme - let's make a release so we get this fix out

@jasongrout
Copy link
Contributor Author

jasongrout commented Jan 21, 2026

It would be pandas v3.0.0 I bet. The timing agrees with when it started failing, and the changeset here looks relevant.

Yep, comparing the logs between a failing and succeeding run, the difference is pandas 3.0 is in the failing run, pandas 2.3.3 in the run that succeeded.

@jasongrout
Copy link
Contributor Author

jasongrout commented Jan 21, 2026 •
edited
Loading

I bet it's that there is a new str type, where the missing value is always NaN. I bet before it was None, which is falsey, but NaN is truthy, so that boolean check used to work, but now failed.

https://pandas.pydata.org/docs/user_guide/migration-3-strings.html#the-missing-value-sentinel-is-now-always-nan

According to the migration guide, the correct fix is to use pd.isna (or notna for the opposite check), so I think this PR does the correct thing.

@jasongrout
Copy link
Contributor Author

jasongrout commented Jan 21, 2026

hah I just ran into this in myst-theme - let's make a release so we get this fix out

+1 - this is blocking us making a jupyterlab release

@choldgraf choldgraf mentioned this pull request Jan 21, 2026
Closed
5 tasks
@choldgraf
Copy link
Member

choldgraf commented Jan 21, 2026 •
edited
Loading

release is already out in the wild!

@jasongrout
Copy link
Contributor Author

jasongrout commented Jan 21, 2026

And our CI tests are now passing with 1.1.5. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nabobalis nabobalis nabobalis approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Started failing with TypeError: '<' not supported between instances of 'str' and 'float'

5 participants

@jasongrout @bsipocz @krassowski @choldgraf @nabobalis